Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: missing authority host param and resource management endpoint when init the client #1648

Merged
merged 10 commits into from
May 28, 2021

Conversation

locmai
Copy link
Contributor

@locmai locmai commented May 28, 2021

Issue

Relates to #1646

Description

This PR fixes 2 parts:

  • First one is adding the AuthorityHost for the TokenCredentialOptions, since this value will be different for each Cloud
  • Second one is similar to the first but with the ResourceGraphClient which will need the baseUri of the Managment Resource Endpoint

@locmai locmai requested a review from tomkerkhove as a code owner May 28, 2021 04:24
@CLAassistant
Copy link

CLAassistant commented May 28, 2021

CLA assistant check
All committers have signed the CLA.

@locmai locmai changed the title Fix passing authority host and ResourceManagerEndpoint fix: missing authority host param and resource management endpoint when init the client May 28, 2021
@tomkerkhove
Copy link
Owner

It looks good, thank you for the PR! I'll wait with merging until you've been able to scrape it but happy to help where I can!

@locmai locmai requested a review from tomkerkhove May 28, 2021 09:39
@locmai
Copy link
Contributor Author

locmai commented May 28, 2021

It looks good, thank you for the PR! I'll wait with merging until you've been able to scrape it but happy to help where I can!

I am able to scrape the metrics and export them to my Prometheus after adding the azureMetadata.cloud in the configmap, the PR for the chart got merged too.

@tomkerkhove
Copy link
Owner

Awesome, thanks a ton @locmai!

@tomkerkhove tomkerkhove merged commit 3073ab3 into tomkerkhove:master May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants